Skip to content

Add 'indirect_sort'#117

Open
mclow wants to merge 7 commits intodevelopfrom
indirectSort
Open

Add 'indirect_sort'#117
mclow wants to merge 7 commits intodevelopfrom
indirectSort

Conversation

@mclow
Copy link
Copy Markdown
Collaborator

@mclow mclow commented Jun 15, 2023

No description provided.

Copy link
Copy Markdown
Contributor

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks potentially useful.

Added some ideas for improving the description and pointed out a few places for improvement/typo-fixing.

Comment thread doc/indirect_sort.qbk Outdated
Comment on lines +12 to +14
There are times that you want a sorted version of a sequence, but for some reason or another, you don't really want to sort them. Maybe the elements in the sequence are non-copyable (or non-movable), or the sequence is const, or they're just really expensive to move around. An example of this might be a sequence of records from a database.

Nevertheless, you might want to sort them. That's where indirect sorting comes in. In a "normal" sort, the elements of the sequence to be sorted are shuffled in place. In indirect sorting, the elements are unchanged, but the sort algorithm returns to you a "permutation" of the elements that, when applied, will leave the elements in the sequence in a sorted order.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this a bit shorter wording especially avoiding to mention the need to sort twice:

Suggested change
There are times that you want a sorted version of a sequence, but for some reason or another, you don't really want to sort them. Maybe the elements in the sequence are non-copyable (or non-movable), or the sequence is const, or they're just really expensive to move around. An example of this might be a sequence of records from a database.
Nevertheless, you might want to sort them. That's where indirect sorting comes in. In a "normal" sort, the elements of the sequence to be sorted are shuffled in place. In indirect sorting, the elements are unchanged, but the sort algorithm returns to you a "permutation" of the elements that, when applied, will leave the elements in the sequence in a sorted order.
There are times that you want a sorted version of a sequence, but for some reason you don't want to modify it. Maybe the elements in the sequence can't be moved/copied, e.g. the sequence is const, or they're just really expensive to move around. An example of this might be a sequence of records from a database.
That's where indirect sorting comes in. In a "normal" sort, the elements of the sequence to be sorted are shuffled in place. In indirect sorting, the elements are unchanged, but the sort algorithm returns a "permutation" of the elements that, when applied, will put the elements in the sequence in a sorted order.

Are the double-spaces after each sentence intended?

Comment thread doc/indirect_sort.qbk Outdated

Nevertheless, you might want to sort them. That's where indirect sorting comes in. In a "normal" sort, the elements of the sequence to be sorted are shuffled in place. In indirect sorting, the elements are unchanged, but the sort algorithm returns to you a "permutation" of the elements that, when applied, will leave the elements in the sequence in a sorted order.

Say you have a sequence `[first, last)` of 1000 items that are expensive to swap:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Say you have a sequence `[first, last)` of 1000 items that are expensive to swap:
Assume a sequence `[first, last)` of 1000 items that are expensive to swap:


#include <algorithm> // for std::sort (and others)
#include <functional> // for std::less
#include <vector> // for std:;vector
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo:

Suggested change
#include <vector> // for std:;vector
#include <vector> // for std::vector

But is that comment really required?

///

#ifndef BOOST_ALGORITHM_IS_INDIRECT_SORT
#define BOOST_ALGORITHM_IS_INDIRECT_SORT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unusual include guard. Why not BOOST_ALGORITHM_INDIRECT_SORT?

/// \fn indirect_sort (RAIterator first, RAIterator las )
/// \returns a permutation of the elements in the range [first, last)
/// such that when the permutation is applied to the sequence,
/// the result is sorted according to the predicate pred.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// the result is sorted according to the predicate pred.
/// the result is sorted in non-descending order.

/// \param last The end of the input sequence
///
template <typename RAIterator>
std::vector<size_t> indirect_sort (RAIterator first, RAIterator last) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::vector<size_t> indirect_sort (RAIterator first, RAIterator last) {
Permutation indirect_sort (RAIterator first, RAIterator last) {

Comment thread test/indirect_sort_test.cpp Outdated


void test_sort () {
BOOST_CXX14_CONSTEXPR int num[] = { 1,3,5,7,9, 2, 4, 6, 8, 10 };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
BOOST_CXX14_CONSTEXPR int num[] = { 1,3,5,7,9, 2, 4, 6, 8, 10 };
int num[] = { 1,3,5,7,9, 2, 4, 6, 8, 10 };

or int *first = &num[0]; is invalid isn't it?

Comment thread test/indirect_sort_test.cpp Outdated

// A permutation of size N is a sequence of values in the range [0..N)
// such that no value appears more than once in the permutation.
bool isa_permutation(Permutation p, size_t N) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool isa_permutation(Permutation p, size_t N) {
bool is_a_permutation(Permutation p, size_t N) {

is more readable.

Comment thread test/indirect_sort_test.cpp Outdated
test_one_sort(v.begin(), v.end(), std::greater<int>());
}

BOOST_AUTO_TEST_CASE( test_main )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why that extra method and not using BOOST_AUTO_TEST_CASE(test_sort) directly?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I expect there to be more test cases in the future.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the whole idea of BOOST_AUTO_TEST_CASE is that you simply "decorate" each test case with that and NOT have a "main" function. By default it will run each such function sequentially even allowing you to filter test based on their name from the CLI.

-->

BOOST_AUTO_TEST_CASE( test_sort ){
...
}

BOOST_AUTO_TEST_CASE( test_indirect_stable_sort ){
...
}

Comment thread test/indirect_sort_test.cpp Outdated
@@ -0,0 +1,100 @@
/*
Copyright (c) Marshall Clow 2011-2012.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Copyright (c) Marshall Clow 2011-2012.
Copyright (c) Marshall Clow 2023.

return ret;
}

/// \fn indirect_partial_sort (RAIterator first, RAIterator last)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C&P mistake in the signature inside this (and below) docstrings

Comment on lines +155 to +158
/// \fn indirect_nth_element (RAIterator first, RAIterator last, Predicate p)
/// \returns a permutation of the elements in the range [first, last)
/// such that when the permutation is applied to the sequence,
/// the result is sorted according to the predicate pred.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar C&P mistake in signature and description.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch - thanks!

@nigels-com
Copy link
Copy Markdown
Contributor

nigels-com commented Apr 12, 2026

I've implemented something similar for a recent project.
A few thoughts to note.

  1. Would like to used uint32_t for "small" jobs less than ~4 billion items
  2. Would like to use this in combination with other sort algorithms such as boost::sort
  3. Would like to use pre-existing permutation as an option, rather than always re-allocate one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants